Skip to content

fix: remove Option from model listing return types, propagate errors#7074

Merged
codefromthecrypt merged 1 commit intomainfrom
fix-model-listing
Feb 9, 2026
Merged

fix: remove Option from model listing return types, propagate errors#7074
codefromthecrypt merged 1 commit intomainfrom
fix-model-listing

Conversation

@codefromthecrypt
Copy link
Collaborator

@codefromthecrypt codefromthecrypt commented Feb 8, 2026

Summary

Remove Option from fetch_supported_models and fetch_recommended_models return types. An empty vec already means "no models" — the Option wrapper just forced every caller to handle None differently.

Additionally, dynamic providers now propagate errors instead of swallowing them into Ok(None). This ensures misconfigured providers (e.g. bad credentials, unreachable host) surface errors to the desktop and CLI rather than silently returning no models.

Type of Change

  • Refactor / Code quality

AI Assistance

  • This PR was created or reviewed with AI assistance

Testing

providers test with ollama:

Details
$ OLLAMA_HOST=http://localhost:11434 cargo test -p goose --test providers -- test_ollama_provider --nocapture                                                
Compiling tonic v0.12.3
Compiling goose-mcp v1.23.0 (/Users/codefromthecrypt/oss/goose-goosed/crates/goose-mcp)
Compiling opentelemetry-proto v0.27.0
Compiling opentelemetry-otlp v0.27.0
Compiling goose v1.23.0 (/Users/codefromthecrypt/oss/goose-goosed/crates/goose)
Finished `test` profile [unoptimized + debuginfo] target(s) in 37.60s
Running tests/providers.rs (target/debug/deps/providers-28809165ae79d804)

running 1 test
=== Ollama::model_listing ===
[crates/goose/tests/providers.rs:374:9] &models = [
"all-minilm:33m",
"all-minilm:l6-v2",
"devstral:latest",
"gemma3:latest",
"llama3.2:latest",
"nomic-embed-text:latest",
"qwen2.5-coder:32b",
"qwen2.5:0.5b",
"qwen2.5:latest",
"qwen3-vl:2b",
"qwen3-vl:latest",
"qwen3:0.6b",
"qwen3:1.7b",
"qwen3:4b",
"qwen3:latest",
]
===================
=== Ollama::reponse1 ===
[crates/goose/tests/providers.rs:154:9] &response1 = Message {
id: None,
role: Assistant,
created: 1770519075,
content: [
Text(
Annotated {
raw: RawTextContent {
text: "",
meta: None,
},
annotations: None,
},
),
ToolRequest(
ToolRequest {
id: "call_p694a081",
tool_call: Ok(
CallToolRequestParams {
meta: None,
name: "get_weather",
arguments: Some(
{
"location": String("San Francisco, CA"),
},
),
task: None,
},
),
metadata: None,
tool_meta: None,
},
),
],
metadata: MessageMetadata {
user_visible: true,
agent_visible: true,
},
}
===================
=== Ollama::reponse2 ===
[crates/goose/tests/providers.rs:203:9] &response2 = Message {
id: None,
role: Assistant,
created: 1770519079,
content: [
Text(
Annotated {
raw: RawTextContent {
text: "The weather in San Francisco is currently **Clear** with a temperature of **50°F (10°C)**. Conditions show **0% precipitation**, **84% humidity**, and **2 mph wind**. This forecast is valid for Saturday at 9:00 PM. ☀\u{fe0f}",
meta: None,
},
annotations: None,
},
),
],
metadata: MessageMetadata {
user_visible: true,
agent_visible: true,
},
}
===================
=== Ollama::context_length_exceeded_error ===
[crates/goose/tests/providers.rs:247:9] &result = Ok(
(
Message {
id: None,
role: Assistant,
created: 1770519089,
content: [
Text(
Annotated {
raw: RawTextContent {
text: "no",
meta: None,
},
annotations: None,
},
),
],
metadata: MessageMetadata {
user_visible: true,
agent_visible: true,
},
},
ProviderUsage {
model: "qwen3",
usage: Usage {
input_tokens: Some(
79,
),
output_tokens: Some(
691,
),
total_tokens: Some(
770,
),
},
},
),
)
===================
Test image not found at crates/goose/examples/test_assets/test_image.png, skipping image test
test test_ollama_provider ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 13 filtered out; finished in 19.35s


============== Providers ==============
✅ Ollama
=======================================

Related Issues

Obviates #7065

Remove the Option wrapper from fetch_supported_models and
fetch_recommended_models. An empty vec already means "no models" so
the Option added complexity with no value.

Dynamic providers now propagate errors instead of swallowing them
into Ok(None). This ensures misconfigured providers surface errors
to the desktop and CLI rather than silently returning no models.

Signed-off-by: Adrian Cole <adrian@tetrate.io>
@codefromthecrypt codefromthecrypt marked this pull request as ready for review February 8, 2026 02:57
Copilot AI review requested due to automatic review settings February 8, 2026 02:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR simplifies provider model-listing APIs by removing Option wrappers (empty Vec now consistently means “no models”) and changes multiple dynamic providers to propagate model-listing errors instead of silently returning no results, so misconfiguration is surfaced to the CLI/desktop/server.

Changes:

  • Update Provider trait model-listing methods to return Result<Vec<String>, ProviderError> and adjust call sites accordingly.
  • Update multiple providers (e.g., OpenRouter, Tetrate, Databricks, LiteLLM) to return errors for request/parse/API failures instead of Ok(None).
  • Update server/CLI/tests and supporting utilities to use the new semantics (empty list vs error).

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
crates/goose/src/providers/base.rs Changes trait defaults and recommended-model filtering to use Vec<String> (empty means none).
crates/goose/src/providers/openrouter.rs Propagates request/parse/API errors; returns Vec<String> directly.
crates/goose/src/providers/tetrate.rs Propagates JSON/shape/API errors; removes “warn + None” fallback.
crates/goose/src/providers/databricks.rs Propagates fetch/parse/status errors; returns Vec<String>.
crates/goose/src/providers/litellm.rs Propagates fetch errors instead of warning + None.
crates/goose/src/providers/openai_compatible.rs Returns Vec<String> and errors on unexpected response shape.
crates/goose/src/providers/openai.rs Returns Vec<String> (no Option).
crates/goose/src/providers/anthropic.rs Returns Vec<String> and errors on missing data array.
crates/goose/src/providers/google.rs Returns Vec<String> and errors on missing models array.
crates/goose/src/providers/githubcopilot.rs Returns Vec<String> and errors on missing data array.
crates/goose/src/providers/ollama.rs Returns Vec<String> (no Option).
crates/goose/src/providers/venice.rs Returns Vec<String> (no Option).
crates/goose/src/providers/gcpvertexai.rs Returns filtered Vec<String> (no Option).
crates/goose/src/providers/lead_worker.rs Combines lead/worker model lists under new non-Option contract.
crates/goose/src/providers/snowflake.rs Adds fetch_supported_models returning known models.
crates/goose/src/providers/bedrock.rs Adds fetch_supported_models returning known models.
crates/goose/src/providers/codex.rs Returns known models as Vec<String> (no Option).
crates/goose/src/providers/chatgpt_codex.rs Returns known models as Vec<String> (no Option).
crates/goose/src/providers/claude_code.rs Adds fetch_supported_models returning known models.
crates/goose/src/providers/gemini_cli.rs Adds fetch_supported_models returning known models.
crates/goose/src/providers/cursor_agent.rs Adds fetch_supported_models returning known models.
crates/goose/src/providers/auto_detect.rs Updates detection logic to treat empty list as “no match”.
crates/goose/src/providers/canonical/build_canonical_models.rs Adapts canonical model build/check tool to new return type.
crates/goose/src/agents/reply_parts.rs Adapts enhanced error messaging to new model-listing semantics.
crates/goose-server/src/routes/config_management.rs Removes declarative-provider static model short-circuit; returns models directly on Ok(Vec).
crates/goose-cli/src/commands/configure.rs Uses “non-empty list => select” else free-text under new API.
crates/goose/tests/providers.rs Updates model listing test to assume Vec<String> return.
Comments suppressed due to low confidence (1)

crates/goose-server/src/routes/config_management.rs:387

  • Removing the declarative-provider fast path changes this endpoint from returning the static config.models list to returning a 400 when the provider isn’t configured, which can break UI flows that rely on model names being available before credentials/network access; consider restoring a fallback for ProviderType::Declarative/Custom when load_provider succeeds (at least for requires_auth == false or when a static models list is present).
pub async fn get_provider_models(
    Path(name): Path<String>,
) -> Result<Json<Vec<String>>, ErrorResponse> {
    let all = get_providers().await.into_iter().collect::<Vec<_>>();
    let Some((metadata, provider_type)) = all.into_iter().find(|(m, _)| m.name == name) else {
        return Err(ErrorResponse::bad_request(format!(
            "Unknown provider: {}",
            name
        )));
    };
    if !check_provider_configured(&metadata, provider_type) {
        return Err(ErrorResponse::bad_request(format!(
            "Provider '{}' is not configured",
            name
        )));
    }

Copy link
Collaborator

@michaelneale michaelneale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this makes a lot of sense to me - I don't know exactly what flow on effects are and not hand tested, but this has always frustrated me - just need to put one in most of the time.

@codefromthecrypt codefromthecrypt added this pull request to the merge queue Feb 9, 2026
Merged via the queue into main with commit 08b89ca Feb 9, 2026
24 checks passed
@codefromthecrypt codefromthecrypt deleted the fix-model-listing branch February 9, 2026 00:12
tlongwell-block added a commit that referenced this pull request Feb 9, 2026
* origin/main: (55 commits)
  test(mcp): add image tool test and consolidate MCP test fixtures (#7019)
  fix: remove Option from model listing return types, propagate errors (#7074)
  fix: lazy provider creation for goose acp (#7026) (#7066)
  Smoke tests: split compaction test and use debug build (#6984)
  fix(deps): trim bat to resolve RUSTSEC-2024-0320 (#7061)
  feat: expose AGENT_SESSION_ID env var to extension child processes (#7072)
  fix: add XML tool call parsing fallback for Qwen3-coder via Ollama (#6882)
  Remove clippy too_many_lines lint and decompose long functions (#7064)
  refactor: move disable_session_naming into AgentConfig (#7062)
  Add global config switch to disable automatic session naming (#7052)
  docs: add blog post - 8 Things You Didn't Know About Code Mode (#7059)
  fix: ensure animated elements are visible when prefers-reduced-motion is enabled (#7047)
  Show recommended model on failture (#7040)
  feat(ui): add session content search via API (#7050)
  docs: fix img url (#7053)
  Desktop UI for deleting custom providers (#7042)
  Add blog post: How I Used RPI to Build an OpenClaw Alternative (#7051)
  Remove build-dependencies section from Cargo.toml (#6946)
  add /rp-why skill blog post (#6997)
  fix: fix snake_case function names in code_execution instructions (#7035)
  ...

# Conflicts:
#	scripts/test_subrecipes.sh
lifeizhou-ap added a commit that referenced this pull request Feb 9, 2026
* main:
  added build notify (#6891)
  test(mcp): add image tool test and consolidate MCP test fixtures (#7019)
  fix: remove Option from model listing return types, propagate errors (#7074)
jh-block added a commit that referenced this pull request Feb 9, 2026
* origin/main: (54 commits)
  chore: strip posthog for sessions/models/daily only (#7079)
  tidy: clean up old benchmark and add gym (#7081)
  fix: use command.process_group(0) for CLI providers, not just MCP (#7083)
  added build notify (#6891)
  test(mcp): add image tool test and consolidate MCP test fixtures (#7019)
  fix: remove Option from model listing return types, propagate errors (#7074)
  fix: lazy provider creation for goose acp (#7026) (#7066)
  Smoke tests: split compaction test and use debug build (#6984)
  fix(deps): trim bat to resolve RUSTSEC-2024-0320 (#7061)
  feat: expose AGENT_SESSION_ID env var to extension child processes (#7072)
  fix: add XML tool call parsing fallback for Qwen3-coder via Ollama (#6882)
  Remove clippy too_many_lines lint and decompose long functions (#7064)
  refactor: move disable_session_naming into AgentConfig (#7062)
  Add global config switch to disable automatic session naming (#7052)
  docs: add blog post - 8 Things You Didn't Know About Code Mode (#7059)
  fix: ensure animated elements are visible when prefers-reduced-motion is enabled (#7047)
  Show recommended model on failture (#7040)
  feat(ui): add session content search via API (#7050)
  docs: fix img url (#7053)
  Desktop UI for deleting custom providers (#7042)
  ...
lifeizhou-ap added a commit that referenced this pull request Feb 11, 2026
* main: (85 commits)
  Fix 'Edit In Place' and 'Fork Session' features (#6970)
  Fix: Only send command content to command injection classifier (excluding part of tool call dict) (#7082)
  Docs: require auth optional for custom providers (#7098)
  fix: improve text-muted contrast for better readability (#7095)
  Always sync bundled extensions (#7057)
  feat: Add tom (Top Of Mind) platform extension (#7073)
  chore(docs): update GOOSE_SESSION_ID -> AGENT_SESSION_ID (#6669)
  fix(ci): switch from cargo-audit to cargo-deny for advisory scanning (#7032)
  chore(deps): bump @isaacs/brace-expansion from 5.0.0 to 5.0.1 in /evals/open-model-gym/suite (#7085)
  chore(deps): bump @modelcontextprotocol/sdk from 1.25.3 to 1.26.0 in /evals/open-model-gym/mcp-harness (#7086)
  fix: switch to windows msvc (#7080)
  fix: allow unlisted models for CLI providers (#7090)
  Use goose port (#7089)
  chore: strip posthog for sessions/models/daily only (#7079)
  tidy: clean up old benchmark and add gym (#7081)
  fix: use command.process_group(0) for CLI providers, not just MCP (#7083)
  added build notify (#6891)
  test(mcp): add image tool test and consolidate MCP test fixtures (#7019)
  fix: remove Option from model listing return types, propagate errors (#7074)
  fix: lazy provider creation for goose acp (#7026) (#7066)
  ...
Tyler-Hardin pushed a commit to Tyler-Hardin/goose that referenced this pull request Feb 11, 2026
Tyler-Hardin pushed a commit to Tyler-Hardin/goose that referenced this pull request Feb 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants